Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixed SSL IP address verification #244

Merged
merged 1 commit into from
May 23, 2024
Merged

Fixed SSL IP address verification #244

merged 1 commit into from
May 23, 2024

Conversation

hikui
Copy link
Contributor

@hikui hikui commented May 22, 2024 •
edited
Loading

Problem description

Our server connects to MongoDB via an IP address. We created an SSL certificate and added the ip address in. However the MongoDB driver fails to verify the host name.

After digging deep into the code, I found that the driver sets server_name_indication for IP addresses, which leads to wrong ReferenceID being tagged as :dns_id. According to https://github.com/erlang/otp/blob/OTP-27.0/lib/public_key/src/public_key.erl#L2771, this will not match any IP address in the certificate.

This PR added IP address detection and skip setting SNI when IP address is presented.

Experiments

Experiment 1:

defmodule SSLConn do
  def pkix_verify_hostname(fqdn, host) do
    IO.inspect("#{inspect(fqdn)}, #{inspect(host)}")
    :default
  end

  def connect() do
    host = ~c"127.0.0.1"
    port = 27018

    {:ok, socket} = :gen_tcp.connect(host, port, [:binary, active: false, packet: :raw, nodelay: true])

    :ssl.connect(socket,
      cacertfile: "/home/hmiao/Workspace/mongo-ca.pem",
      verify: :verify_peer,
      customize_hostname_check: [match_fun: &pkix_verify_hostname/2],
      server_name_indication: ~c"127.0.0.1"
    )
  end
end

Output:

"{:dns_id, ~c\"127.0.0.1\"}, {:iPAddress, [127, 0, 0, 1]}"
{:error,
 {:tls_alert,
  {:handshake_failure,
   ~c"TLS client: In state wait_cert at ssl_handshake.erl:2140 generated CLIENT ALERT: Fatal - Handshake Failure\n {bad_cert,hostname_check_failed}"}}}

Experiment 2

defmodule SSLConn do
  def pkix_verify_hostname(fqdn, host) do
    IO.inspect("#{inspect(fqdn)}, #{inspect(host)}")
    :default
  end

  def connect() do
    host = ~c"127.0.0.1"
    port = 27018

    {:ok, socket} = :gen_tcp.connect(host, port, [:binary, active: false, packet: :raw, nodelay: true])

    :ssl.connect(socket,
      cacertfile: "/home/hmiao/Workspace/mongo-ca.pem",
      verify: :verify_peer,
      customize_hostname_check: [match_fun: &pkix_verify_hostname/2],
    )
  end
end

Output

"{:ip, {127, 0, 0, 1}}, {:iPAddress,  [127, 0, 0, 1]}"
{:ok,
 {:sslsocket, {:gen_tcp, #Port<0.11>, :tls_connection, :undefined},
  [#PID<0.372.0>, #PID<0.371.0>]}}

@hikui hikui force-pushed the ssl-verification-fix branch 2 times, most recently from cd910c4 to 602b917 Compare May 22, 2024 09:38
@hikui hikui force-pushed the ssl-verification-fix branch from 602b917 to 3b9585f Compare May 22, 2024 10:11
@zookzook zookzook self-assigned this May 23, 2024
@zookzook
Copy link
Owner

Thank you for your clear and understandable description.

@zookzook zookzook merged commit 1f5ee7d into zookzook:master May 23, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@zookzook zookzook

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hikui @zookzook